Skip to content

fix(web): restore empty-query skill discovery#353

Open
codeblackhole1024 wants to merge 1 commit intoiflytek:mainfrom
codeblackhole1024:fix-search-empty-query-regression
Open

fix(web): restore empty-query skill discovery#353
codeblackhole1024 wants to merge 1 commit intoiflytek:mainfrom
codeblackhole1024:fix-search-empty-query-regression

Conversation

@codeblackhole1024
Copy link
Copy Markdown

Summary

  • restore the search URL helper behavior so empty queries do not send a blank q parameter
  • keep empty-query search pages aligned with the homepage discovery requests
  • update regression tests for empty-query discovery behavior

Verification

  • ./node_modules/.bin/vitest run src/shared/hooks/skill-query-helpers.test.ts src/pages/search.test.tsx
  • ./node_modules/.bin/vite build

@CLAassistant
Copy link
Copy Markdown

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

Copy link
Copy Markdown
Collaborator

@dongmucat dongmucat left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

改动方向没问题,空查询不发 q 参数更干净,后端对 q= 和不传 q 的处理也是等价的(PostgresFullTextQueryService.normalizeKeyword 对 null 和空串都返回 null,走 discovery 逻辑)。

但有一个需要同步处理的地方:

E2E 断言和实现冲突

web/e2e/search-page-full.spec.tsTC_SEARCH_INPUT_003(L44-58)还在等待一个必须包含空 q 的请求:

return response.status() === 200
  && url.searchParams.has('q')
  && url.searchParams.get('q') === ''

这次改动之后 buildSkillSearchUrl({ q: '' }) 不再发送 q 参数,这个 E2E 用例会超时失败。

建议把这个断言改成验证"空查询仍然能拿到 discovery 结果"的用户可见行为,而不是钉死请求里必须带空 q。比如只检查请求命中了 /api/web/skills 且不包含非空 q,然后断言页面上有 skill card 渲染出来就行。

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants